Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add title to input method #29

Merged
merged 2 commits into from
Feb 26, 2018
Merged

Conversation

pjaudiomv
Copy link
Collaborator

would this work

@pjaudiomv
Copy link
Collaborator Author

How do I update my fork master to your master

@dgershman
Copy link
Collaborator

Go to the folder on your local device, from the command line.

  1. Add the upstream repo as a remote
    git remote add upstream https://github.com/radius314/yap

  2. Rebase your commits on top of the prior history from upstream (If there are conflicts, you may have to go down a different path).
    git rebase upstream/master

  3. If server does not consider the timezone of caller #2 is successful.
    git push origin master -f

input-method.php Outdated
@@ -5,6 +5,7 @@
echo "<?xml version=\"1.0\" encoding=\"UTF-8\"?>";

$searchType = $_REQUEST['Digits'];
$inputTitle = $_REQUEST['Title'];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already a "Title" configuration value. We should use that, but instead specify that we want to play the title here. Also you need to check to make sure this value is set to 1, otherwise it will bomb out.

$playTitle = isset($_REQUEST['PlayTitle']) ? $_REQUEST['PlayTitle'] : 0;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be sure to also add the documentation of this feature in README.md.

input-method.php Outdated
@@ -14,7 +15,8 @@
?>
<Response>
<Gather numDigits="1" timeout="10" action="input-method-result.php?SearchType=<?php echo $searchType ?>" method="GET">
<Say voice="<?php echo $voice; ?>" language="<?php echo $language; ?>"><?php echo $inputTitle ?></Say>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is option, you should wrap this in an if block.

Per my previous comment pull the title from the respective configuration value.

input-method.php Outdated
@@ -5,6 +5,7 @@
echo "<?xml version=\"1.0\" encoding=\"UTF-8\"?>";

$searchType = $_REQUEST['Digits'];
$inputTitle = $_REQUEST['Title'];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be sure to also add the documentation of this feature in README.md.

@pjaudiomv
Copy link
Collaborator Author

alright made those changes, and it works not sure if its pretty enough but it works.

input-method.php Outdated
echo "<Say voice=\"" . $voice . "\" language=\"" . $language . "\">" . $GLOBALS['title'] . "</Say>";
}
?>

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. The only thing would be to fix indentations. Notice how I try to keep things lined up for readability. https://github.com/radius314/yap/blob/master/meeting-search.php

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha, thanks. im starting to get more of a grasp on this whole github, fork, push, pull, commit thing too I think.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@dgershman dgershman merged commit 6d8c51f into bmlt-enabled:master Feb 26, 2018
@dgershman
Copy link
Collaborator

Just tested, works good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants